Show user fields dialog again if upload fails #1415
Conversation
AlbyIanna
requested review from
ubidefeo,
per1234,
davegarthsimpson and
francescospissu
|
Is there a chance to move the entire user-fields logic to another module and let |
per1234
added
type: enhancement
Yes, it would make sense. I'll work on it later. |
|
@kittaakos please have a look at the last commit, I've created a separate contribution for the user fields. |
| @injectable() | ||
| export class UserFields extends Contribution { | ||
| private boardRequiresUserFields = false; | ||
| private userFieldsSet = false; |
There was a problem hiding this comment.
Can we not simplify this and if there are user fields for an fqbn in the map, it means true. Otherwise, false.
There was a problem hiding this comment.
In that we would lose the value of the user field. As far as I know, we don't want that, we want to preserve the value, but give the user the possibility to change it.
| @@ -258,7 +163,7 @@ export class UploadSketch extends CoreServiceContribution { | |||
| if (!CurrentSketch.isValid(sketch)) { | |||
| return undefined; | |||
| } | |||
| const userFields = this.userFields(); | |||
| const userFields = this.userFields.getUserFields(); | |||
There was a problem hiding this comment.
Why not pass the fqbn from here?
There was a problem hiding this comment.
I don't see any reason since I can easily get them from the boardsServiceProvider. Passing it here would just add a parameter, with no benefit.
There was a problem hiding this comment.
Exactly this; why do you want to calculate it twice? verify, and upload will require the fqbn anyway.
There was a problem hiding this comment.
IMO how the user fields are stored is not related to the upload. I may want to open that dialog even if I'm not uploading.
AlbyIanna
force-pushed
the
show-user-fields-dialog
branch
from
b00d70d to
94ca5f3
Compare
Motivation
When uploading a sketch with user fields, if the user made a mistake while filling in the dialog, or made a change that requires updating the fields, it might not be clear to them how to do that 🙁
Change description
As requested in #1386, I've made a change to show again the user field dialog when pressing the Upload button if the previous upload failed.
Other information
Closes #1386
Reviewer checklist